-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SetStatusIcon #19
base: main
Are you sure you want to change the base?
Conversation
Ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and sorry about the delay (I was busy, then sick, etc.).
I've left some minor questions and comments. Overall, this looks very reasonable, and we can merge after addressing these points.
switch m.(type) { | ||
case *image.NRGBA: | ||
invertImageNrgba(m.(*image.NRGBA)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? Can the type be something other than *image.NRGBA
? If so, what can it be? Is doing nothing in those cases the best thing to do?
|
||
// TODO: Implement. | ||
void set_status_item_icon(struct image img) {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra blank line at the end, it's not needed.
@@ -56,6 +56,13 @@ func Initialize(title string, imageData []byte, items []MenuItem) { | |||
} | |||
} | |||
|
|||
// SetStatusIcon replaces the icon image data. | |||
func SetStatusIcon(imageData []byte) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we already have trayhost.UpdateMenu
to modify the menu items, it might make more sense to use more consistent naming.
How about UpdateIcon
?
Refs #18
Use demonstrated at https://github.com/tmc/trayhost-demo/tree/master/osx